Pandas 1 fixes#268
Merged
nbokulich merged 4 commits intoqiime2:masterfrom Feb 14, 2020
Merged
Conversation
Please see these links for the rational if interested: pandas-dev/pandas#13443 pandas-dev/pandas#23752
This makes it easier to see what's going on below with the values returned by this function.
This prevents attempting to drop columns that don't exist in merged.columns after setting the index, while still dropping columns that are present in merged.columns. Attempting to do so raises an exception in pandas >= 1. Please see pandas-dev/pandas#8594 for details.
andrewsanchez
commented
Feb 13, 2020
| # Removes the column name used to set the index of `merged` above | ||
| col_diff = set(columns) - set([column]) | ||
| if col_diff: | ||
| counts.drop(col_diff, axis=1, inplace=True, level=0) |
Contributor
Author
There was a problem hiding this comment.
The first kwarg taken by DataFrame.drop is labels and it accepts a single label or list-like. Passing the set directly seems to work fine here, but we can make it a pd.Index or list if that is preferable.
This avoids attempting to drop columns that had already been dropped in previous calls to _reindex_with_metadata in the for loop in `alpha_rarefaction`. Co-authored-by: Matthew Dillon <matthewrdillon@gmail.com>
andrewsanchez
commented
Feb 13, 2020
| counts = merged.count() | ||
| counts.drop(columns, axis=1, inplace=True, level=0) | ||
| median_ = merged.median() | ||
| reindexed = merged.set_index(column) |
Contributor
Author
There was a problem hiding this comment.
Avoid mutating merged by assigning to a new DataFrame reindexed.
nbokulich
requested changes
Feb 14, 2020
Member
nbokulich
left a comment
nbokulich
left a comment
There was a problem hiding this comment.
Thanks @andrewsanchez ! please see my inline comment.
Member
|
Thanks for clarifying... LGTM |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.